feat(auth): add Kimi and Xiaomi MiMo providers - #8368

E2E Test Report

Environment

• macOS, Node.js 24
• Repository source CLI via QWEN_SANDBOX=false npm run dev -- --screen-reader
• Terminal size: 100×24
• Isolated temporary Qwen settings directory

Baseline

The globally installed Qwen Code 0.21.3 did not expose Kimi or Xiaomi MiMo under /auth → Third-party Providers; both required manual Custom Provider setup. The expanded descriptive provider list also had no item-window pagination tied to the dialog height.

Post-change result

• Opened /auth → Third-party Providers and confirmed ▲/▼ scroll affordances with the footer continuously visible.
• Repeated Down input moved the visible viewport from DeepSeek/Grok to Idealab/Kimi and then to Requesty/Xiaomi MiMo without clipping.
• Opened Kimi and confirmed one top-level provider with Coding Plan, API Key (China), and API Key (International) choices.
• Confirmed Coding Plan and API endpoints expose their separate recommended model sets.
• Opened Xiaomi MiMo and confirmed pay-as-you-go plus China, Singapore, and Europe Token Plan choices with MiMo V2.5 models.
• Backed out without submitting or persisting any real credentials.

Result

✅ Passed on macOS.

✅ Qwen Triage finished — view run. See the stage comments in this thread for the result.

✅ Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

Re-run gate at 3c2c1331 — after the round-19 Critical fixes (f9d446f8), the CI repair push (3c2c1331), and the maintainer's second real-environment verification of this exact head.

Template: looks good ✓

Problem: real user friction, not a theoretical concern — Kimi and Xiaomi MiMo users had to hand-recreate these endpoints through Custom Provider, and Kimi's Coding Plan vs regional APIs use different credential variables and different model catalogs, so a flat preset would install invalid models for some endpoints. The pagination fix addresses a real clipping bug: the maintainer reproduced the corruption on the base build at 100×24 and confirmed the fix on this PR's build.

Direction: aligned. First-class third-party presets are an established pattern here — the CHANGELOG records "Added xAI Grok as a built-in model provider preset" (#6805) alongside DeepSeek, MiniMax, ModelScope and others. Extending the existing preset seam to carry endpoint-specific metadata is the minimal way to express subscription-scoped endpoints.

Size: core provider paths touched across seven packages. ~1,850 production-logic lines vs ~5,750 test lines (~1:3 ratio), ~75 doc lines, ~3 CI lines. That is far past the 500-line core threshold, so the Stage 0 maintainer-awareness escalation applies; it has been thoroughly exercised — 20 review rounds, a takeover loop, and maintainer live verification ending in an approval at this commit. The 1000+ line large-PR advisory applies too, but splitting an already-converged review would cost more than it saves — informational only.

Approach: the endpoint metadata rides on the existing preset seam (BaseUrlOption.models + a resolver + endpoint-scoped metadata keys) instead of a parallel mechanism, and all six client surfaces consume that one source. Alphabetical ordering and height-based pagination are small and contained. Nothing unrelated to the stated goal stands out.

Risk: packages/cli/src/acp-integration/ matches a revert-correlated path pattern, so no review depth was skipped. The actual change there is confined to provider catalog/config serialization — no agent or connection lifecycle changes — and it is covered by unit tests, green CI, and the maintainer's live drive.

Moving on to code review. 🔍

— Qwen Code · qwen3.8-max

_{Reviewed at 3c2c1331aeae2b0f1dee44861801628d942f6207 · re-run with @qwen-code /triage}

Code review (re-run at 3c2c1331)

Independent proposal: before reading the diff I'd have done this: two new preset files following the existing preset pattern; since Kimi's three access types and MiMo's four endpoints need different base URLs, env keys and model catalogs, extend BaseUrlOption with optional per-endpoint models plus a resolver mapping (provider, baseUrl) → models; scope providerMetadata keys per endpoint so sibling installs don't overwrite each other, with a legacy-key fallback for existing presets; thread the metadata through serve/ACP/VS Code/web-shell/desktop; fix the list-height pagination.

Comparison: the PR matches this shape almost exactly and goes further where it matters. resolveProviderModels + BaseUrlOption.models is the seam; resolveMetadataKey gains a stable provider--endpoint suffix (with . escaping against dotted-path writes); the version hash now tracks the provider's built-in template rather than the user's selection, so a deselected default never re-triggers the update prompt; and applyModelProvidersPatch replaces models in place at the first removed position instead of prepending, so re-submitting one endpoint can't silently reorder or delete a sibling. Secrets handling improved along the way: the ACP/desktop wire types replaced a serializable existingConfig.apiKey with a boolean hasApiKey, and the serve catalog degrades per-provider (a throwing derivation only costs that provider its derived field, not the whole catalog).

Critical blockers: none found. This head already absorbed 20 review rounds; the round-19 Criticals were fixed in f9d446f8 and the duplicate-import CI breakage in 3c2c1331.

Non-blocking observations:

• Six round-20 Suggestions remain open (the automated reviewer's last pass, all severity S). After 20 rounds the right move per the anti-balloon guidance is to defer them to a follow-up rather than spin another round. The two most substantive: the endpoint-restore edge cases in AuthDialog (saved models without a baseUrl fold into the restored endpoint; a saved baseUrl matching no shipped option falls back to provider-wide defaults — data-preserving, just coarse), and the near-duplicate endpoint-state helpers in the desktop and web-shell packages (apiKeyAfterBaseUrlChange & co.), which live in separate packages with different wire types and are hard to share today.
• In kimi.ts the endpoint documentation URLs appear both in the baseUrl options and again in the top-level documentationUrl function — harmless, but the function could look the option up to keep one source.
• Follow-up candidates (both confirmed by the maintainer's verification): ModelIdsStep still clips below ~30 terminal rows — pre-existing, reproduces on the base build; and the legacy providerMetadata.<id> key is left behind after migration to the endpoint-suffixed key — harmless, never read again for multi-endpoint providers.

Testing

CI evidence (this commit's own runs, via API — no PR code was executed by triage):

All pull_request-event workflow runs on this commit are complete; nothing is pending. The three skipped test jobs are merge-queue-only by workflow design, not a gap in this PR. Not verified by triage: the macOS/Windows platform legs and the integration suite never run at PR stage for any PR — they will run in the merge queue.

Behavioural claims. The central claims are behavioural (endpoint routing, per-endpoint model lists, migration of existing installs), and they were verified on this exact head by the maintainer's real-environment report above in this thread: live CLI TUI + qwen serve + Web Shell against the real Kimi/MiMo endpoints, per-endpoint 401 wire proof, and an Alibaba-legacy upgrade path that migrated with a byte-identical result. That is maintainer evidence, not something this triage run re-executed — unattended CI never runs PR code. The sandboxed verification run triggered by the same command (run 31608893329) is still in flight and will post its own report here; if a bot-driven TUI capture is also wanted, a maintainer can trigger the isolated @qwen-code /tmux job.

— Qwen Code · qwen3.8-max

_{Reviewed at 3c2c1331aeae2b0f1dee44861801628d942f6207 · re-run with @qwen-code /triage}

Confidence: 3/5 — clean re-review at 3c2c1331 with no open Criticals; the cap is pure policy, not doubt — a fork feat touching core infrastructure at this scale (~1,850 production lines across seven packages) is Stage 0's maintainer-escalation case, and the gate never casts the deciding approval on one of those.

Stepping back: the implementation matches or exceeds my independent proposal at every point, the test-to-production ratio is roughly 3:1, and the design chose the hard-but-right options — template-based version hashes, endpoint-scoped ownership, in-place merge positions, hasApiKey instead of key values on the wire. Twenty review rounds plus a takeover loop have converged it: the round-19 Criticals are fixed, CI on this head is green, and the remaining round-20 items are Suggestions that belong in a follow-up, not another round. The maintainer then verified it end to end in a real environment at this exact commit — endpoint routing, model lists, and the legacy-Alibaba upgrade path — and approved it.

⏸️ Deferring to @wenshao — policy cap, no code blockers. Because Stage 0 escalated this PR for maintainer awareness, the triage gate does not approve it regardless of how clean the stages look; the merge decision stays human. Concretely, two housekeeping items stand between this PR and merge:

1. The standing CHANGES_REQUESTED in the review decision is a bot review anchored at an older commit (e9a7eba5, Aug 11) — seventeen fix-rounds and a green CI run ago. The bot's latest pass at the current head is COMMENTED, so a maintainer may need to dismiss that stale review before branch protection will allow merging.
2. main requires two approving reviews; @wenshao's approval at 3c2c1331 is one. If the second must also be human, that is the remaining sign-off.

If it were my call on the code alone, this would be a 4/5 approve — the two held-back points are the policy cap and the open Suggestions backlog, neither of which reflects doubt about what is on this commit.

— Qwen Code · qwen3.8-max

_{Reviewed at 3c2c1331aeae2b0f1dee44861801628d942f6207 · re-run with @qwen-code /triage}

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head 92e345e. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

⚠️ No preview: one or more scenarios failed to render on this head — see the workflow run. This is not "no visual change" — a scenario that times out or throws produces no image. Fix the failing scenario (or a genuine regression it caught) and the preview returns on the next push.

_{Full-resolution recordings (.webm) are attached to the workflow run.}

— Qwen Code · web-shell visuals

🩺 serve daemon A/B

Built the PR base vs this PR head 92e345e, drove a fixed endpoint set against each, and diffed the JSON responses. Only fields that changed are shown.

✅ No response changes against the PR base across 12 scenario(s).

— Qwen Code · serve A/B

Review: feat(auth): add Kimi and Xiaomi MiMo providers

Overview

Two new third-party presets (kimiProvider, xiaomiMimoProvider) plus a genuinely useful generalization: BaseUrlOption.models lets a single preset carry per-endpoint model catalogs. The new resolveProviderModels(config, baseUrl) is threaded through buildModelConfigs / buildProviderTemplate / getDefaultModelIds in core, and then consistently through all five setup surfaces (CLI /auth, ACP, qwen serve/daemon, VS Code, Web Shell, desktop). The third-party registry is sorted alphabetically, and AuthDialog gained height-aware pagination. Design doc + sources are included.

The plumbing is thorough — I checked every getDefaultModelIds( call site and the baseUrl-less ones (AuthDialog.tsx:226, useProviderUpdates.ts:181,244) are all deliberately the "all built-in IDs" use, so nothing was missed. Comments below are mostly about edge cases and the implicit invariants the new shape introduces.

Correctness

1. selectBaseUrl discards user input when the same endpoint is re-confirmed — packages/cli/src/ui/auth/useProviderSetupFlow.ts:222

The handler unconditionally rewrites modelIds and apiKey on every endpoint selection. Repro: pick an endpoint → type an API key (or tick/untick models) → Esc back to the endpoint step → Enter on the same endpoint. The typed key is replaced by existingProviderEnv[envKey] ?? '' and the model edits are rebuilt from defaults. This affects every multi-endpoint preset (MiniMax, Alibaba Token Plan, …), not just Kimi. Suggest gating on an actual change:

if (provider && selectedUrl !== baseUrl) { /* reset models + key */ }

Web Shell has the same issue and no env restore — AuthMessage.tsx:475 and :551 call setApiKey('') unconditionally, so re-confirming the endpoint always wipes the typed key. Desktop (onValueChange) is fine since Select only fires on change.

2. builtInIds is computed three different ways — CLI vs desktop vs web shell

• useProviderSetupFlow.ts:227 → provider.models only
• ProviderConnectForm.tsx modelIdsAfterBaseUrlChange → provider.models ∪ all baseUrl[].models
• AuthMessage.tsx baseUrlOptionModelIds → provider.models ∪ all baseUrl[].models

The CLI version is only correct today because kimiProvider.models happens to be [...KIMI_CODE_MODELS, ...KIMI_API_MODELS]. Nothing enforces that invariant — a future preset that lists models only on its BaseUrlOptions would make the CLI classify the other endpoint's built-ins as "user custom IDs" and carry them across endpoints. Worth (a) exporting one core helper (getAllProviderModelIds(config)) and using it in all three places, and (b) documenting on BaseUrlOption.models that ProviderConfig.models must remain the union.

3. getDefaultModelIds(config) vs getDefaultModelIds(config, url) now mean different things — provider-config.ts:361

Omitting baseUrl returns the union across endpoints; passing one returns that endpoint's list. Both meanings are used intentionally in this PR, but neither function has a doc comment saying so. Given how easy it is to drop the argument, please add JSDoc to resolveProviderModels/getDefaultModelIds.

Related: with an unmatched baseUrl, resolveProviderModels falls through resolveBaseUrl to config.baseUrl[0] and returns the first option's models, not config.models. Probably intended (it mirrors what the install would actually use), but it's surprising from the name — worth a comment.

4. Kimi's two endpoints can't coexist — presets/kimi.ts:71

ownsKimiModel matches both [Kimi Code]/KIMI_CODE_API_KEY and [Kimi API]/MOONSHOT_API_KEY, and buildInstallPlan uses prepend-and-remove-owned. So installing Coding Plan after the Open Platform API (or vice versa) removes the other endpoint's models, and providerMetadata.kimi.baseUrl only records the last one. A user who has both will also see the auto-update prompt in useProviderUpdates offering to remove 4 models when only the other endpoint's list changed. Unlike MiniMax/Token Plan (shared credential + shared catalog), these two are separate subscriptions with separate keys, so coexistence is plausible. Either set mergeModelsByIdentity: true, or state in the design doc that the access types are mutually exclusive.

5. Wire-protocol semantics change for descriptor.models — auth-provider-helpers.ts:70, acpAgent.ts:2019

Top-level models / defaultModelIds now resolve against the first base-URL option instead of provider.models. For Kimi, daemon/ACP clients see only the 4 Coding Plan IDs at the top level. Any existing client using descriptor.models as "the provider's full catalog" (e.g. to split built-in vs custom IDs) will now misclassify. Please document the new meaning on ServeAuthProviderDescriptor.models, DaemonAuthProviderDescriptor.models, and QwenProviderSummary.models — the field is only additive-safe for the per-option models, not for this one.

UI / pagination

6. Fixed-row constants don't cover the error banner — AuthDialog.tsx:131

MAIN_LIST_FIXED_ROWS = 10 is measured for the error-free main view, but authError/errorMessage adds rows, so the exact case the pagination was added to fix can still clip. Also, because the reservation is large, dialogHeight ≤ 18 collapses the 3-item main menu to a single visible item (floor((18-10-4)/3) = 1). Consider dropping the separator + ToS block when height-constrained instead of shrinking the list to one row — that chrome is what's consuming the budget.

7. Theme color is frozen at module load — AuthDialog.tsx:51

MAIN_ITEMS is a module-level constant whose descriptions are now JSX reading theme.text.secondary. theme is a live getter proxy (semantic-colors.ts), but the value is captured once when the module is imported at CLI startup. Previously the description was a plain string and DescriptiveRadioButtonSelect applied the live theme at render time, so this is a small regression: change the theme, reopen /auth, and the three main-menu descriptions keep the startup color. (providerToItem is fine — the dialog unmounts on close, so its useMemo(…, []) re-runs.) Building MAIN_ITEMS inside the component, or keeping the string and letting the list component apply wrap="truncate", both fix it.

Tests

Good additions: preset metadata/install-plan tests, registry ordering, serve-helper serialization, endpoint-scoped update in useProviderUpdates, ACP list payload.

Gaps and nits:

• The riskiest new CLI logic is untested. selectBaseUrl's model/API-key reset (issue 1 & 2) has no unit test. A renderHook test over useProviderSetupFlow covering "switch endpoint → defaults swap, custom IDs preserved" and "re-select same endpoint → typed key preserved" would pin both.
• AuthDialog.test.tsx — the third-party test flipped from "shows the presets" to not.toContain('MiniMax API Key') / not.toContain('Z.AI API Key'). Nothing now asserts every third-party preset is reachable. Consider keeping a THIRD_PARTY_PROVIDERS-driven assertion (list membership, not frame text) and adding a scroll-down test that Kimi/Xiaomi MiMo become visible — the PR body verifies that manually only.
• expect(frame?.split('\n')).toHaveLength(17) is brittle against any layout change; asserting "no more than the budget" would be more durable.
• createSettings() duplicates the inline LoadedSettings construction already present later in the same file — worth reusing one helper.
• all-providers.test.ts asserts the declaration order of ALL_PROVIDERS is alphabetical. That's a reasonable lint, but note the sort key is the English label while the UI renders t(config.label) — under a locale that translates labels the displayed order won't be alphabetical.
• Model metadata (k3-256k = 262,144 tokens, mimo-v2.5-pro text-only, which IDs are thinkingMandatory) can't be verified from the repo. Since a wrong thinkingMandatory silently changes request shape, it'd be good to have a second reviewer confirm against the linked docs.

Style / conventions

• Presets match the existing deepseek.ts/grok.ts/minimax.ts shape; license headers, exported *_ENV_KEY constants, and uiGroup are all consistent. 👍
• serializeProviderModel extraction in auth-provider-helpers.ts is a clean dedup.
• Minor asymmetry: label: 'Kimi' vs 'Xiaomi MiMo API Key' / 'MiniMax API Key'. Justified (Kimi isn't API-key-only), just noting it — the docs list already reflects it.
• serializeProviderModel drops thinkingMandatory and imageOnly; pre-existing for top-level models, but it now also applies to the per-option lists, so remote clients never see those flags. Fine since the install resolves server-side — just confirming it's intentional.

Security / performance

Nothing concerning. API keys keep the existing env-var storage path, are cleared rather than leaked on endpoint switch, and aren't logged. THIRD_PARTY_PROVIDERS.sort() runs once at module load on a filter() copy (doesn't mutate ALL_PROVIDERS); resolveProviderModels is a small linear scan behind useMemo.

Summary

Solid, well-scoped feature with the cross-surface plumbing done carefully. The blocking-ish items for me are #1 (input loss when re-confirming an endpoint — user-visible and easy to hit) and #2/#3 (the unenforced "models is the union" invariant plus the two meanings of getDefaultModelIds, which is the kind of thing the next preset will get wrong). #4 and #5 are decisions to make explicit rather than defects. The rest are polish.

Review round 2: feat(auth): add Kimi and Xiaomi MiMo providers

Re-reviewed the full diff at c55523f, focused on what fix(auth): address endpoint provider review feedback changed since round 1.

What the follow-up commit fixed ✅

• KIMI_CODE_API_KEY / MOONSHOT_API_KEY / MIMO_API_KEY wired into ci.yml + no-ak-integration-ci.test.js — good catch, this was missing.
• clearPersistedAuth now clears function-derived env keys (settingsWriter.ts:685), so a Kimi logout actually removes both keys instead of neither.
• Per-endpoint envKey added to the serve/daemon catalog + web shell review preview, so Kimi's review screen names the right variable.
• buildProviderSetupInputs exported and unit-tested; useProviderSetupFlow.test.ts added.

New issues introduced by the follow-up commit

A. The envKey resolution regresses the Custom Provider descriptor — packages/cli/src/serve/server/auth-provider-helpers.ts:52

const envKey =
  typeof provider.envKey === 'function'
    ? provider.envKey(provider.protocol, defaultBaseUrl)
    : provider.envKey;
...
...(envKey ? { envKey } : {}),

buildAuthProviderCatalog maps all of ALL_PROVIDERS, and customProvider has baseUrl: undefined → resolveBaseUrl(provider) returns '' → this calls generateCustomEnvKey(USE_OPENAI, '') and emits a real-looking but meaningless QWEN_CUSTOM_API_KEY_OPENAI__<hash-of-empty-url> on the descriptor.

The old guard (typeof provider.envKey === 'string') omitted it, and web shell fell back correctly. Now selectedBaseUrlEnvKey (AuthMessage.tsx:108) hits provider.envKey ?? … first, so the Custom Provider review preview shows the wrong env var name — previously OPENAI_API_KEY / ANTHROPIC_API_KEY / GEMINI_API_KEY per the chosen protocol, now a hash key that will never exist. The persisted config is still correct (the install resolves server-side), but the preview lies to the user.

Suggested fix — only derive the top-level key when the provider actually has a fixed endpoint:

const envKey =
  typeof provider.envKey === 'string'
    ? provider.envKey
    : provider.baseUrl !== undefined
      ? provider.envKey(provider.protocol, defaultBaseUrl)
      : undefined;

The per-option envKey mapping below it is fine as-is (custom provider has no options).

B. clearPersistedAuth's new loop runs the custom provider through an empty base URL — settingsWriter.ts:689

const baseUrls = Array.isArray(p.baseUrl) ? … : [p.baseUrl ?? ''];

Same '' path: for customProvider this derives and deletes 3 hash keys that can never exist (one per protocolOptions entry). Harmless but confusing given the comment two lines below already says custom-provider keys are swept by prefix. if (p.baseUrl === undefined) continue; before the loop makes the intent explicit.

C. Kimi is invisible to GenAI telemetry — packages/core/src/telemetry/gen-ai-provider.ts:22,44

MIMO_API_KEY / XIAOMI_MIMO_API_KEY and xiaomimimo.com are already mapped, so MiMo is covered. Kimi is not: neither MOONSHOT_API_KEY nor KIMI_CODE_API_KEY is in PROVIDER_BY_ENV_KEY, and providerFromHostname has no case for kimi.com / moonshot.cn / moonshot.ai. resolveGenAiProviderName therefore falls through to authType: 'openai' and every Kimi request is attributed to provider openai in telemetry. Two lines in each table fixes it (moonshot is the semconv name).

D. Doc links are dead for both new presets

auth-provider-helpers.ts:61 only serializes documentationUrl when it's a string. Kimi and MiMo use the function form, so daemon/web-shell clients get no documentation link at all — every other preset shows one. Separately, ACP resolves it once against defaultBaseUrl (acpAgent.ts:2020), so desktop always shows the Kimi Code docs even after the user picks the Moonshot endpoint.

Meanwhile the per-option documentationUrl you added in kimi.ts / xiaomi-mimo.ts is never read anywhere: BaseUrlSelectStep (ProviderSetupSteps.tsx:60) ignores it, and nothing serializes it. Every other multi-endpoint preset (minimax, zai, alibaba-*) puts docs only on the option and has no provider-level function — I'd match that and either drop the function form, or teach the two serializers to resolve per-endpoint. As written it's two sources of truth, both unused on some surfaces.

E. Test-timing band-aids in shared helpers — AuthDialog.test.tsx:187,195,203,205

pressEnterAndWaitFor and moveDownAndWaitForSelection gained unconditional setTimeout(50) on both sides, plus extra await wait() at half a dozen call sites. These are shared helpers used across the whole file, so this adds fixed latency to every navigation test and papers over whatever race actually needed it rather than naming it. If the real problem is that ink hasn't flushed the frame before the next keypress, a vi.waitFor on the selected row (which waitForSelectedOption already does) should be sufficient — worth a comment explaining what the sleep is waiting for if it has to stay.

Related: the new loop drives Down through ['Grok (xAI) API Key', 'Idealab API Key', 'Kimi', 'MiniMax API Key'] to reach MiniMax. That hard-codes the alphabetical registry order, so the next preset added between D and M breaks an unrelated test. A "press Down until MiniMax API Key is selected, max N times" loop is order-independent.

Still open from round 1

• pre-release: fix ci #1 — selectBaseUrl discards typed input when the same endpoint is re-confirmed (useProviderSetupFlow.ts:223). The handler still resets unconditionally; web shell (AuthMessage.tsx:473, :549) still calls setApiKey('') unconditionally with no env restore. Repro is unchanged: pick endpoint → type key → Esc → Enter on the same endpoint → key gone. A selectedUrl !== baseUrl guard is still the smallest fix. This was my main blocking item.
• Where is the config saved? #2 — three different builtInIds computations. useProviderSetupFlow.ts:227 still uses provider.models only, while desktop and web shell union it with all baseUrl[].models. Still correct only because kimiProvider.models happens to be the union. One exported core helper would settle it.
• 如何自定义密钥文件 .env可能与其他文件冲突 #3 — no JSDoc on resolveProviderModels / getDefaultModelIds. The with-arg vs without-arg meanings still differ silently, and the unmatched-baseUrl fallback (returns the first option's models via resolveBaseUrl, not config.models) is still undocumented.
• Are you interested in AI Terminal? #4 — Kimi's two access types can't coexist (ownsKimiModel matches both prefixes + prepend-and-remove-owned). Still no mergeModelsByIdentity and no note in the design doc.
• TypeError in Authentication Selection Interface #5 — descriptor.models semantics changed (now the first endpoint's list, not the full catalog) with no doc comment on ServeAuthProviderDescriptor / DaemonAuthProviderDescriptor / QwenProviderSummary.
• OpenAI API Error: 401 Incorecct API Key provided #6 — MAIN_LIST_FIXED_ROWS / SUB_MENU_LIST_FIXED_ROWS still ignore the error banner. I re-counted the chrome: 10 and 7 are exactly right for the error-free view, and authError || errorMessage adds marginTop 1 + at least 1 text row (more when it wraps). Since DialogManager renders this dialog precisely because uiState.auth.authError is set, the clipping this PR fixes returns in the most common failure path. Also worth a comment deriving the constants — right now they're unexplained magic numbers that any chrome change silently invalidates.
• API Key是要设成阿里云的API Key吗？ #7 — theme.text.secondary frozen at module load in MAIN_ITEMS. Unchanged. Adding wrap="truncate" to the string branch of DescriptiveRadioButtonSelect.tsx:75 is a one-line fix that keeps the live theme and helps every other dialog too.

Test coverage

useProviderSetupFlow.test.ts is a good addition and pins the switch-endpoint path (defaults swap, custom IDs preserved, key restored from env). It does not cover the case that's actually broken — re-selecting the same endpoint after typing a key. Worth adding alongside the fix for #1, since a passing switch-endpoint test currently reads as "this area is covered".

buildProviderSetupInputs test and the envKey assertions in auth-provider-helpers.test.ts are well targeted. The latter would have caught issue A with one more line: expect(catalog.providers.find(p => p.id === 'custom-openai-compatible')?.envKey).toBeUndefined().

Security / performance

Nothing new. Keys still live in the env-var path, are cleared rather than leaked on endpoint switch, and aren't logged. The clearPersistedAuth loop is bounded by preset count. No new network calls.

Summary

The follow-up commit closed the CI-env-key and logout gaps cleanly. Remaining blockers for me:

1. A — new regression: bogus envKey on the Custom Provider descriptor, user-visible in the web shell review preview.
2. pre-release: fix ci #1 (round 1) — still loses typed input on endpoint re-confirm, in both CLI and web shell.
3. C — Kimi mis-attributed as openai in telemetry; two-line fix, easy to forget later.

#2/#3 (the unenforced "models is the union" invariant) remain the thing the next preset author will get wrong. D, E, #4–#7 are polish / decisions to make explicit.

Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with @qwen-code /review. See workflow logs.

Round 43 — all 4 [Critical] findings verified against the code and confirmed real. All fixed in 9d74aafc29 (branch head 650769154b, merges upstream f877fb3525).

All four are the same defect family the merge branch already gates: the non-merge code paths (and the /auth dialog) stamp/drop baseUrl-less legacy entries without ever consulting legacyEnvKeyAttribution — the R41-4 fail-closed gate the merge branch enforces via adoptable = owned && (attributable || !namesSiblingEndpoint(model)). Verified impartially: provider-shape claims checked against every preset (minimax/zai/alibabaStandard are exactly the non-merge + array-baseUrl + shared static envKey set; deepseek is the single-endpoint non-merge control), and each witness reproduced by code-path analysis before writing the fixes.

Negative controls first: all new regression tests fail on the pre-fix tree exactly as the probes predict, then pass with the fixes.

• R43-1 (ACP, acpAgent.ts non-merge branch) — real. The branch stamps every baseUrl-less entry with the selected endpoint before any endpoint check; attribution computed ~20 lines above is never consulted. For a shared static key, an explicit qwen/providers/connect omitting the id drops it from preserveModels, and since a non-merge install plan carries the unscoped ownsModel, prepend-and-remove-owned deletes the stored entry; with modelIds omitted it is preserved stamped — silently re-homed, flipped again on the next connect. Fix: entries with !namesSelectedEndpoint && namesSiblingEndpoint are carried through unstamped (return [model]) so they are written back byte-identical — omission alone would still delete them under the unscoped predicate. The ACP route keeps its informed-deselection requestedIds gate otherwise (this route seeds existingConfig, unlike serve).
• R43-2 (serve, run-qwen-serve.ts non-merge branch) — real; serve twin of R43-1, separate copy, same fix applied there.
• R43-3 (dialog, AuthDialog.tsx) — real. Both getExistingProviderSetup's and getProtocolSetups's preserve flatMaps stamped every baseUrl-less entry with the restored endpoint with no attribution gate and no migratedLegacyModelIds — and start() prefers preserveModelsByProtocol, so fixing only one flatMap would leave the re-homing unchanged, exactly as noted. Fix: extracted ONE shared computePreservedModels helper used by both flatMaps; baseUrl-less ids are seeded stamped only when namesSelectedEndpoint holds (with env key following the stamp, R39-6) and emitted as migratedLegacyModelIds (plumbed through start()/protocol-swap/buildCurrentInputs) so the stored original collapses instead of duplicating; shared/sibling keys are unseeded everywhere (also gated out of the models-field seed lists), and for non-merge providers carried through unstamped since their plans' unscoped ownsModel deletes anything that doesn't reach the plan.
• R43-4 (serve non-merge deselection asymmetry) — real. The branch treated absence from an explicit modelIds as deselection, contradicting the R42-1 route-level rationale ~30 lines below in the same function ("the serve catalog exposes no existingConfig, so Web Shell and SDK selections are defaults-seeded and can never carry" saved ids). A stamped same-endpoint custom survived the implicit shape but was deleted by the explicit defaults-only shape this function was rewritten to handle. Fix: dropped the requestedIds gate in the non-merge branch (shouldPreserve = belongsToAnotherEndpoint || !defaultIds.has(preserved.id)), aligning it with the merge branch's merge-only semantics; seeded CLI/ACP/VS flows remain the authoritative deselection surfaces. The ACP twin intentionally keeps its gate (informed route).

Tests (all negative-controlled): serve e2e minimax explicit+implicit arms through buildInstallPlan+applyProviderInstallPlan (entry survives byte-identical, no stamped copy), serve deepseek defaults-only explicit reconnect preserves the stamped custom, ACP connect minimax both arms assert the unstamped carry, AuthDialog unit tests for both views (shared key unseeded/no migrate ids; sibling-attributable kept out of a foreign endpoint seed; attributable stamped + ids emitted), and two full dialog-chain flow tests (getExistingProviderSetup+getProtocolSetups → start() → untouched submit() → buildInstallPlan → apply): the Kimi shared-key entry survives alone and byte-identical, the deepseek attributable entry collapses to its stamped copy.

Verification: negative controls first — all 9 new tests fail on the pre-fix tree exactly as the probes predict, then pass with the fixes. Full regression: run-qwen-serve 280/280, acpAgent 472/472 (incl. a new minimax shape in the suite's mocked findProviderById, same pattern as the R42 kimi injection), AuthDialog 47/48 + the known 2-core TUI input-timing flake (should preserve the selected main entry when returning from each top-level flow — passes standalone; its fixture carries no modelProviders, so the changed code paths are inert in it; same flake class reproduced on clean HEAD in round 39), useProviderSetupFlow 33/33, ProviderSetupSteps/useAuth green, core providers 17 files / 208 passed, tsc --noEmit 0 errors, eslint 0, prettier clean.

One observed residual (not reported this round, recorded for the next review pass): the VS Code companion's AuthMessageHandler non-merge preserve branch (lines ~431-451) has the same missing-gate shape for baseUrl-less legacy entries; its merge branch fails closed by never restoring such entries. Left untouched here per the reported-surface scope of this round.

Round 44 — all 4 [Critical] findings verified against the code and confirmed real. All fixed in f3c457057b. (Posted as one PR-level comment because inline replies remain blocked by the author's pending review 4851233521 / 550 drafts, which I do not touch.)

Each finding reproduces on 650769154b. Below is the per-item evidence, the fix, and the regression test. The branch is also merged up to current origin/main (13 commits; one acpAgent.test.ts conflict resolved by keeping both sides).

R44-1 — ownsModel endpoint-match clause degenerates at selectedEndpoint === '' (provider-config.ts). REAL.
normalizeBaseUrlForMatching(undefined) returns '', so a free-form install whose resolved baseUrl is '' made clause 1 evaluate '' === '' for every baseUrl-less entry and claim it, short-circuiting the attribution guard. Serve reaches this: parseAuthProviderBaseUrl(undefined) passes and buildProviderSetupInputs resolves to '' with no getDefaultBaseUrlForProtocol fallback (the ACP twin throws when still empty).
Fix: the endpoint-match clause now requires model.baseUrl !== undefined, so baseUrl-less entries are only owned through the attribution-guarded branch.
Test: an empty-baseUrl free-form install keeps the sibling / suffix-less / floating witness entries (all previously deleted).

R44-2 — R42-1 merge-only guarantee void for the free-form provider on serve (run-qwen-serve.ts). REAL.
For the free-form provider only, buildInstallPlan's env-key clause (freeFormProvider && ownsLegacyEnvKey) owns attributable baseUrl-less entries. An explicit-modelIds serve reconnect (the only shape Web Shell sends) dropped such an entry from preserveModels, and the env-key clause then deleted it.
Fix: added ProviderSetupInputs.roundTrippedLegacyModelIds; the env-key clause now treats omission as deselection only for ids the caller exposed (or that the plan writes). Serve passes [] — its catalog exposes no existingConfig, so absence is never deselection there. I used this plan-level signal rather than the suggested serve-side shouldPreserve rewrite because that rewrite would also preserve/stamp array-provider (KIMI) entries and regress the R42-1 byte-identical-survival tests; the round-trip gate scopes the change to the free-form env-key clause and leaves preserveModels untouched.
Test: an explicit serve reconnect at the entry's own endpoint keeps the attributable entry.

R44-3 — dialog migratedLegacyModelIds emission lets the id-collision claim delete a never-migrated floating entry (useProviderSetupFlow.ts / provider-config.ts). REAL.
The id-collision clause migratedLegacyModelIds.has(model.id), gated only by !namesSiblingEndpoint, claimed a baseUrl-less entry whose key names NO endpoint (floating prefix-only key) whenever its id collided with a migrated entry — namesSiblingEndpoint cannot protect a key that names no endpoint.
Fix: the id-collision clause additionally requires namesSelectedEndpoint(model). Callers only migrate attributable entries, so no real migration is under-claimed, but a floating entry is no longer reached by a colliding id.
Test: a floating entry sharing an id with a migrated attributable entry survives while the attributable twin still collapses.

R44-4 — dialog hides attributable baseUrl-less entries yet the plan claims them (AuthDialog.tsx). REAL.
When the first saved entry has no baseUrl, initialBaseUrl resolves to '', so the R43-3 attribution gate keeps the entry out of the seed; the env-key clause still owned it on submit (its stored key matched the typed endpoint), silently deleting a model the user was never shown. Deletion depended on array ordering in settings.
Fix: via the same roundTrippedLegacyModelIds mechanism, the dialog emits the ids it actually surfaced, so omission is deselection intent only for those. An attributable entry the dialog could not restore/expose is protected from the env-key claim; a surfaced-and-deselected entry is still removed (the bot's "suppress the claim for entries the dialog never exposed" option).
Tests: an unexposed attributable entry is protected; an exposed attributable entry that is deselected is still removed.

Verification (all under the build lock). With the fix applied, all four new R44 regression tests pass and the pre-existing suites stay green:

• run-qwen-serve.test.ts: 281/281 (incl. R44-2 regression).
• acpAgent.test.ts: 477/477.
• core src/providers: 212/213 — the 1 failure is supportsImageGeneration from the merged upstream commit (code I did not touch).
• CLI auth suites (AuthDialog / useProviderSetupFlow / useAuth / ProviderSetupSteps): 113/114 — the 1 failure is the documented 2-core TUI-timing flake (itWhenTuiInputReliable, skipped on CI), unrelated to this change (the dialog change only adds a data field to the submit inputs).

Negative control. Each regression test encodes the corresponding bot probe. On the unfixed code that probe deletes the entry — R44-1: survivor sib: 0 / sfx: 0 / flt: 0; R44-2: explicit arm survivors for my-custom: 0; R44-3: floating F deleted while the attributable twin collapses; R44-4: my-model survivor = undefined. Each test asserts the post-fix survival/collapse and passes with the fix applied (serve 281/281, core providers 212/213 including all four R44 tests).

Unrelated failures, noted for transparency (neither is in code this PR changed):

• core providers 1/213 — supportsImageGeneration comes from the merged upstream commit (f1b1305a76, "dual-role image generation models"); it surfaced only under the heavy 17-file parallel run on this oversubscribed host.
• CLI auth 1/114 — the documented 2-core TUI-timing flake in the itWhenTuiInputReliable navigation tests (skipped on CI, where CI=true); this round's dialog change only appends a data field to the submit inputs and touches no rendering/keyboard path.

Round-45 response — all 7 [Critical] items verified line-by-line against the code at 7afbf33; all seven are genuine (several are regressions/edge-cases in our own round-36..44 machinery, as anticipated). All fixed in f9178ecc8b (branch head 16806044cc merges origin/main). Each fix has a regression test that fails on the pre-fix tree.

R45-1 — readExistingProviderConfig seeds unattributable baseUrl-less ids (all 3 surfaces). Confirmed. The endpoint-scoped (array-merge) path has the namesSelectedEndpoint gate, but the non-scoped path seeded every baseUrl === undefined model unconditionally into the top-level modelIds, the free-form modelIdsByBaseUrl reduce, and the per-protocol byEndpoint loop. The desktop client submits that seed back verbatim as an explicit selection, the connect handler adopts+stamps it, but buildInstallPlan can never claim a floating key → permanent duplicate. Fix: compute legacyEnvKeyAttribution(config, protocol, baseUrl) once (and per protoBaseUrl in the protocol loop) and gate every baseUrl-less seed on namesSelectedEndpoint via a shared seedableRestoredLegacy predicate — mirroring the CLI dialog gate. Attributable ids are still seeded so an echoed seed keeps its R42-1 deselection meaning. Tests: acpAgent.test.ts "does not seed floating baseUrl-less legacy ids on a free-form provider (R45-1)".

R45-2 — merge branch adopts floating baseUrl-less entries but can never claim them. Confirmed (ACP and its serve twin). adoptable = owned && (attributable || !namesSiblingEndpoint) lets a floating key through, and the id was pushed to migratedLegacyModelIds, but the id-collision claim is gated on migratedLegacyModelIds.has(id) && namesSelectedEndpoint(model) — which a floating key can never satisfy — so the stamped copy was written beside the unclaimed original (verified: 2 entries pre-fix). Fix: thread explicitly-adopted floating ids through a new, dedicated adoptedFloatingModelIds channel on ProviderSetupInputs; buildInstallPlan claims them in the baseUrl-less clause, kept distinct from migratedLegacyModelIds so the R44-3 over-claim guard (a floating entry whose id merely collides with a migrated attributable entry) stays intact. Applied to both the ACP route and the serve twin. Tests: core install.test.ts (claim + the guard) and acpAgent.test.ts "(R45-2)".

R45-3 — free-form baseUrl Input onBlur early-returns without setBaseUrl. Confirmed. When canonicalBaseUrl reconciles to the committed endpoint the handler returned without committing the canonical text, so handleSubmit (which submits the raw baseUrl) persisted the trailing-slash / userinfo variant. Fix: exactly the suggested change — still setBaseUrl(nextBaseUrl) when the canonical form differs from the field state before returning.

R45-4 — VS Code non-merge branch stamps/deletes fail-closed shared-key entries. Confirmed. This is the AuthMessageHandler gap the round-43/44 notes already flagged as a known residual; minimax/zai/alibaba-standard (non-merge, array baseUrl, one shared static key) fail attribution closed, yet isSelectedEndpointModel returned true purely on baseUrl === undefined, so the accept path re-homed the entry and the deselect path deleted it (unscoped non-merge ownsModel). Fix: apply legacyEnvKeyAttribution in the models step and carry baseUrl === undefined && !namesSelectedEndpoint && namesSiblingEndpoint entries through UNSTAMPED (return [model]) before the default/selection checks — mirroring ACP/serve/CLI. Test: AuthMessageHandler.test.ts "(R45-4)".

R45-5 — VS Code restore path seeds+stamps floating entries it can never claim. Confirmed (same root as the round-44 blocker on this file). For merge free-form, endpointScoped is false so every baseUrl-less entry passed the filter; the R34-4 fixture is precisely such a floating entry. It was stamped into preserveModels but the stored original is claimable only via ownsLegacyEnvKey or migratedLegacyModelIds+attribution — a floating key satisfies none → both entries persisted. Fix: gate the restored/seeded set on namesSelectedEndpoint (floating entries are never seeded or stamped), and for attributable entries now stamped, pass their ids as migratedLegacyModelIds so the install plan collapses the stored original. The R34-4 test is updated accordingly (its fixture was a floating key; it now uses an attributable key and asserts the collapse), plus a new floating test. Tests: AuthMessageHandler.test.ts "(R34-4 × R45-5)" and "(R45-5)".

R45-6 — non-merge update flow conflates baseUrl-less legacy entries with sibling entries. Confirmed. modelsAtBaseUrl can't see a baseUrl-less entry (normalizeBaseUrlForMatching(undefined) = ''), so the diff reported installed built-ins as additions, while the preserve filter kept them → a stamped template copy written beside each baseUrl-less built-in, permanently. Base replaced them cleanly. Fix: for non-merge single-endpoint (string baseUrl) providers, count an owned baseUrl-less entry as belonging to the selected endpoint in both the preserve filter (so the template replaces it) and getInstalledOwnedModelIds (so the diff sees it as installed). Array-baseUrl providers keep the safe preserve behavior (a baseUrl-less entry can't be attributed to one region). Test: useProviderUpdates.test.ts "(R45-6)".

R45-7 — stripUserInfo truncates the authority at the first /. Confirmed (reproduced side-by-side): a basic-auth password containing / (or ?/#) delimits the authority before the @, so the consumer returned the URL with userinfo intact while the producer's sanitizeProviderBaseUrl strips it via the whole-string last-@ fallback when new URL() throws. Fix: ported the producer's algorithm verbatim (scheme/authority parse → new URL attempt → guarded whole-string last-@ fallback via findUnescapedUserInfoFallbackAt). Test: provider-state.test.ts "(R45-7)".

Verification (all under the shared build lock, re-run after merging origin/main): core install 53, acpAgent 474, run-qwen-serve 282, useProviderUpdates 54, VS Code AuthMessageHandler 16, desktop provider-state 38 — all green; the new regression tests fail on the pre-fix tree (negative controls).

Merge note: origin/main (27 commits) was merged into the branch. This brought moonshotProvider (#9814) which overlaps this PR's Kimi API endpoints on MOONSHOT_API_KEY; the telemetry/provider-registry conflicts were resolved by deferring to main's dedicated moonshot attribution (api.moonshot.ai/cn → moonshot, kimi.com coding → kimi) and inserting moonshotProvider in label-sorted registry position. A pre-existing upstream-merge type error in serve/workspace-providers-status.test.ts (configOptions) is untouched by this PR and left as-is.

Round-46 response — both Critical threads verified genuine and fixed in 6432e8e.

R46-1 (all-providers.ts:69) — credential-space collision: confirmed. Reproduced at prior HEAD: findProviderByCredentials was a plain first-match find over ALL_PROVIDERS; kimiProvider registers ahead of moonshotProvider while deriving the same MOONSHOT_API_KEY for the same api.moonshot.{ai,cn}/v1 URLs, so the lookup resolved to kimi and upstream moonshot.test.ts failed with expected 'kimi' to be 'moonshot' (core provider suite: 1/222 failing). Our own kimi.test.ts simultaneously claimed the same lookup for kimi — the two suites contradicted each other. This also conflicted with the round-45 adjudication that already attributes MOONSHOT_API_KEY to the upstream moonshot provider in telemetry.

Fix: credential discovery is now deterministic instead of order-dependent — a preset declaring the env key statically is the canonical owner of that credential space and beats presets that only derive the same key via a function envKey. Moonshot wins the shared space (matching the telemetry attribution); Kimi keeps its regional API endpoints selectable and uniquely owns the KIMI_CODE_API_KEY Coding Plan space. Registry/label order stays untouched, so the label-sort invariant still holds. Pinned by a new collision test in all-providers.test.ts; kimi.test.ts assertions flipped to the adjudicated owner.

R46-2 (AuthDialog.test.tsx:1846) — walk-list omission: confirmed. Reproduced at prior HEAD: after 'Kimi', the frame shows › Kimi (Moonshot AI) API Key selected and the walk times out waiting for 'MiniMax API Key' (gated by itWhenTuiInputReliable, so it only failed outside CI). The missing label is inserted at its alphabetical position.

Verification at new head 6432e8e: core provider suite 223/223 (was 221/222, plus one new regression test); AuthDialog.test.tsx 48/48; adjacent consumers (systemInfoFields, AppHeader, useProviderUpdates, useProviderSetupFlow, ProviderSetupSteps, doctorChecks, auth-provider-helpers) 125/125; run-qwen-serve.test.ts 282/282; core tsc --build clean (typechecks the edited test files). Note: tsc --noEmit for packages/cli OOMs in this 2-core/3.5 GB review environment (pre-existing infra limit, not code); the only cli-side change is a single string literal in a test file that runs green, and CI typecheck remains authoritative.